-
-
Notifications
You must be signed in to change notification settings - Fork 282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] MBS-13561 / MBS-13571 / MBS-13818 / MBS-13580: Show primary aliases in more places #3410
Draft
reosarevok
wants to merge
10
commits into
metabrainz:master
Choose a base branch
from
reosarevok:MBS-13561
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to be able to load aliases even in cases where there is no need to load all the rest of the related info for an artist, such as in tag pages. That means it makes sense to extract load_aliases as a separate method that can be called by load_related_info. While we only use this in artists right now, there's no big reason that it needs to be that way. As such, this moves it out of Data::Artist into the Alias role so it can be used by other entities as well.
The aliases and primary_alias properties added to Artist alone will be useful for other entities as well, so this extracts them into a new Entity::Role::Alias role and adds the role to all the entity types that support aliases.
We already translate instruments, so showing a primary alias for them would just be redundant.
This loads the primary aliases for /tag/x and /tag/x/entity_type pages (for all entity types that support them).
This loads the primary aliases for /user/x/subscriptions/type pages (for all entity types that support them).
Rather than doing my $lang = $c->stash->{current_language} // 'en'; a lot of times on different files, this sets a current_language attribute in Server that we can then just call every time we need it.
This loads the primary aliases during load_entities for relationships, so that for example related works and writers will show the aliases from a work page. Still missing: pass $c->current_language from the controller rather than trying to get it in data which cannot work.
This was not failing right now, but it will when we start loading aliases for ACs in a subsequent commit. I also cannot see any reason to try it.
We never actually pass a hover, so all passedHover was doing here was complicate things for no reason. I dropped it. This makes it simpler to merge the two cases where we show disambiguation on hover, which makes my brain hurt a bit less (marginally) and will make it easier to show primaryAlias in a subsequent commit.
This loads the primary aliases for artist credit artists so that they can be displayed on hover instead of the sortnames when looking at each artist in the credit. Still missing: pass $c->current_language from the controller rather than trying to get it in data which cannot work.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement MBS-13561 / MBS-13571 / MBS-13818 / MBS-13580
Description
Shows primary aliases in a lot more places. See commits and tickets for details.